Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: Rename 'py' to 'pytest' in GHA to avoid confusion #749

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: Rename 'py' to 'pytest' in GHA to avoid confusion #749

wants to merge 1 commit into from

Conversation

MaxymVlasov
Copy link
Collaborator

@MaxymVlasov MaxymVlasov commented Jan 9, 2025 •
edited
Loading

Description of your changes

When you see py in tests it not clear what exactly going on. Until now I thought that it represents "python", but actually this part run pytest
image

So, for avoiding confusion and add ability to explicitly specify tox r -e pytest, this PR was made

How can we test changes

Wait till tests pass on this PR

@MaxymVlasov MaxymVlasov requested a review from yermulnik as a code owner January 9, 2025 22:45
@yermulnik
Copy link
Collaborator

Wow, 56 tests 😲

I also think we should ask @webknjaz to also review and approve Py-based PRs here to get his knowledge work for us 😄

@MaxymVlasov
Copy link
Collaborator Author

Rename at least in GHA view works fine, but some tests failed, idk why. Rerun doesn't help
@webknjaz I see that these test runs on ubuntu 24.04, when prev were on 22.04. Could it be the reason why it failed? In other 2 PRs for test also used ubuntu 24.04, but their tests passed 🤔

@webknjaz
Copy link
Contributor

The "Download all the dists" step got skipped and the following job couldn't install the expected wheel artifact for testing, crashing the job. The rest were auto-cancelled due to fail-fast in the matrix.

@webknjaz
Copy link
Contributor

It's possible their past platform issues haven't been fully resolved: https://www.githubstatus.com/history. Give it time and restart in the morning.

@webknjaz
Copy link
Contributor

The root cause is that this part hasn't yet been made reusable: https://github.com/antonbabenko/pre-commit-terraform/blob/5181957/.github/workflows/reusable-tox.yml#L265C52-L265C56. You'll need to add the new name to the hardcoded list there, for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yermulnik yermulnik yermulnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MaxymVlasov @yermulnik @webknjaz